Skip to content

BC5 readiness: pairwise BC4↔BC5 + check-bc5-compat orchestrator + scheduled CI#308

Merged
jeremy merged 42 commits into
mainfrom
bc5-readiness-pairwise-orchestrator
Jul 22, 2026
Merged

BC5 readiness: pairwise BC4↔BC5 + check-bc5-compat orchestrator + scheduled CI#308
jeremy merged 42 commits into
mainfrom
bc5-readiness-pairwise-orchestrator

Conversation

@jeremy

@jeremy jeremy commented May 16, 2026

Copy link
Copy Markdown
Member

Summary

Closes the BC5-readiness canary loop: PRs 1–3 built the per-backend live capture (TS) and cross-language decode (Ruby/Python/Go/Kotlin) infrastructure. PR 4 adds the pairwise BC4↔BC5 comparison that catches the additive-only invariant — and the orchestrator + scheduled CI that runs the whole pipeline end-to-end on a daily cron.

Stacked on #301 (PR 3 — cross-language wire-replay decoders). Once #301 merges, this PR retargets main.

Why pairwise

Per-backend schema validation alone can't detect a class of regression: with every new BC5 field marked optional, a case where BC4 emits memories: [items] and BC5 emits memories: [] passes each backend's schema independently — yet that's exactly the additive-only invariant the canary should catch. This is not hypothetical: BC5 ships precisely this divergence — now codified as documented contract (see below).

The canonical canary rule lives on GetMyNotifications. The intended invariant: BC5 memories[] should remain a superset of BC4's. But BC5 ships json.memories [] while BC4 (the four branch) still populates it — and bc3 has settled this as a permanent, documented contract divergence: doc/api/sections/my_notifications.md (bc3 #11628) codifies memories as an always-empty placeholder superseded by bubble_ups, and the once-planned alias (json.memories @bubble_ups, bc3 #10947) never shipped — #10947 is closed unmerged. So the pairwiseSupersetArray: ["memories"] rule stays as the encoded additive-only invariant, permanently waived by the pairwiseDeltaAllowed: ["memories"] entry — the machine-readable record of the accepted BC4→BC5 subtractive delta, tracked in spec/api-gaps/memories-emptied-regression.md and indexed by the public compatibility report. Retire the waiver only if BC4 empties memories too (the superset rule then passes clean) or BC5's documented contract changes.

What's in this PR

  • conformance/schema.jsonpairwiseAssertions[] with four rule types: pairwiseSupersetArray, pairwiseSupersetKeys, pairwiseEqual, pairwiseDeltaAllowed (with required reason so a future public compatibility report can index accepted divergences).
  • conformance/tests/live-my-surface.json — the memories SupersetArray rule + a top-level SupersetKeys rule on GetMyNotifications.
  • scripts/compare-canary-runs.sh — applies pairwise rules to a pair of snapshot directories. Exit 0 clean / 1 violation / 2 operator error.
  • Makefilemake conformance-live (one backend's capture + 4 replays) and make check-bc5-compat (BC4 pass + BC5 pass + pairwise). Both opt-in; not in make check.
  • .github/workflows/live-canary.yml — nightly cron + workflow_dispatch, gated on repo secrets (no-ops with a log message when missing rather than failing the run). Uploads snapshots as a 14-day artifact so failures can be inspected post-hoc without rerunning.
  • CONTRIBUTING.md — new "Pairwise BC4↔BC5 comparison", "Orchestrator", and "Scheduled CI" subsections under "Live canary".

Path syntax (dotted identifiers on each snapshot)

  • "" (empty string) addresses the body root.
  • foo.bar defaults to pages[0].body.foo.bar — the common single-page case.
  • pages[N].body.X addresses a specific page.
  • pages[*].body.X aggregates across pages into a list (each page's value becomes one element; useful when you want a per-page count, not a sum of inner lengths).

Test plan

  • jq empty conformance/schema.json — clean JSON
  • jq empty conformance/tests/live-my-surface.json — clean JSON
  • bash -n scripts/compare-canary-runs.sh — shell syntax clean
  • Synthetic-fixture test: pairwiseSupersetArray flags BC5 memories shorter than BC4
  • Synthetic-fixture test: pairwiseSupersetKeys flags BC5 missing a top-level key BC4 has
  • Synthetic-fixture test: pairwiseEqual flags value divergence
  • Synthetic-fixture test: pairwiseDeltaAllowed waives a flagged path
  • Synthetic-fixture test: pages[*].body aggregation collects per-page values
  • make -n check-bc5-compat resolves correctly
  • actionlint .github/workflows/live-canary.yml clean
  • zizmor .github/workflows/live-canary.yml clean
  • make validate-api-gaps clean
  • make check-bucket-flat-parity clean
  • Real-backend run — requires BASECAMP_TOKEN/ACCOUNT_ID/BC5_HOST; CI will be the first opportunity once secrets are configured.

Operational requirement

Pairwise rules assume identical account state across the BC4 and BC5 runs. The same project list, same notifications, etc. Without that, naturally-drifting collections (unreads, etc.) will false-fail rules. CONTRIBUTING.md calls this out explicitly; the canary needs a dedicated test account with stable, equivalent fixtures replicated to each backend before scheduled CI can pass meaningfully.

Sequencing

After this lands, the BC5-readiness canary infrastructure is feature-complete (PRs 1–4). Remaining BC5 readiness work:

  • PR 5scripts/detect-api-gaps.sh + weekly cron workflow (api-gap detection tooling; validation already shipped in PR 1).
  • PR 6 — Go wrapper forward-compat audit + drift check (filed separately; unblocks basecamp-cli Phase 1).
  • PR 7+ — BC3 contract-drift handoff passes (5 passes targeting main).

Summary by cubic

Adds pairwise BC4↔BC5 comparison with a check-bc5-compat orchestrator, nightly CI, and a hardened compare script + Makefile backed by a regression harness. The canonical memories rule stays encoded and is permanently waived via pairwiseDeltaAllowed to reflect BC5’s documented always‑empty memories; violation output redacts live values, path grammar is strictly validated, and pages[*] compares item totals.

  • New Features

    • conformance/schema.json: pairwiseAssertions[] with pairwiseSupersetArray, pairwiseSupersetKeys, pairwiseEqual, pairwiseDeltaAllowed; paths require at least one entry; waivers require non‑empty reason.
    • Orchestrator: make conformance-live and make check-bc5-compat; .github/workflows/live-canary.yml (nightly + manual, gated on secrets, defaults BASECAMP_HOST to https://3.basecampapi.com, uploads decode‑only artifacts; excludes wire/).
    • conformance/tests/live-my-surface.json: GetMyNotifications carries top‑level pairwiseSupersetKeys and a SupersetArray rule on memories, permanently waived via pairwiseDeltaAllowed (BC5’s documented always‑empty memories).
    • Regression suite: make check-compare-canary runs scripts/test-compare-canary-runs.sh (bash ≥ 4.4, jq required; uses portable grep -E on BSD/macOS).
  • Bug Fixes

    • Compare script: exact TS snapshot filename mapping; missing snapshots hard‑fail (exit 2); validates snapshot shape (including per‑page key types) and recorded operation; strict path grammar (only dotted segments or leading pages[N]/pages[*] into .body; no hyphens or jq punctuation; rejects bare [] streams and non‑leading [*]); shorthand paths are forbidden once either capture paginates; pages[N] must exist on both; pages[*] arrays compare total item counts; pairwiseSupersetKeys rejects non‑object targets and does not accept pages[*]; pairwiseEqual uses deep equality and fully redacts live values in messages.
    • Rule/waiver validation: tests file must be a top‑level array; reject unknown rule types up‑front (before waiver skips); rule paths must be non‑empty arrays of strings; waivers require non‑empty reason and non‑empty paths; waiver paths validated like enforcing paths (waivers do not bypass path checks).
    • Makefile/CI/docs: absolutizes LIVE_RECORD_DIR; safe cleanup with segment‑aware .. checks, checkout‑ancestor refusal, full trailing‑slash strip, refusal when the canonicalized path is the filesystem root, and rm -rf -- on the original path; GNU make 3.81‑safe env defaults; fast env preflight; check-compare-canary wired into make check; CI artifacts upload decode results only and exclude raw wire/.

Written for commit 881138e. Summary will update on new commits.

Review in cubic

Copilot AI review requested due to automatic review settings May 16, 2026 17:17
@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/live-canary.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

@github-actions github-actions Bot added documentation Improvements or additions to documentation github-actions Pull requests that update GitHub Actions conformance Conformance test suite labels May 16, 2026
@github-actions github-actions Bot added enhancement New feature or request and removed documentation Improvements or additions to documentation labels May 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the missing “BC4 ↔ BC5 additive-only invariant” layer to the live canary by introducing pairwise assertions, a snapshot comparison script, an end-to-end Makefile orchestrator, and a nightly GitHub Actions workflow to run the pipeline.

Changes:

  • Extend the conformance test schema + live surface test fixture with pairwiseAssertions rules (superset arrays/keys, equality, and allowlisted deltas).
  • Add scripts/compare-canary-runs.sh to apply pairwise rules across two backend snapshot directories.
  • Add Makefile orchestration targets (conformance-live, check-bc5-compat) and a scheduled workflow that runs the canary and uploads artifacts.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
scripts/compare-canary-runs.sh New pairwise snapshot comparator that enforces additive-only invariants between BC4 and BC5.
Makefile Adds orchestration targets to run BC4 pass, BC5 pass, then pairwise compare.
conformance/schema.json Adds schema support for pairwiseAssertions rule definitions.
conformance/tests/live-my-surface.json Adds canonical pairwise rules for GetMyNotifications (memories array + top-level keys).
.github/workflows/live-canary.yml Nightly + manual workflow to run make check-bc5-compat and upload snapshots as artifacts.
CONTRIBUTING.md Documents pairwise comparison semantics, orchestrator usage, and scheduled CI behavior.
COORDINATION.md Minor update reflecting that PR4 plan item is now landed here.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/compare-canary-runs.sh Outdated
Comment thread scripts/compare-canary-runs.sh Outdated
Comment thread scripts/compare-canary-runs.sh
Comment thread conformance/schema.json
Comment thread Makefile Outdated
Comment thread .github/workflows/live-canary.yml Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread Makefile Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a663ad9301

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/compare-canary-runs.sh Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 7 files

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread scripts/compare-canary-runs.sh Outdated
Comment thread scripts/compare-canary-runs.sh Outdated
Comment thread .github/workflows/live-canary.yml Outdated
Comment thread Makefile Outdated
Comment thread scripts/compare-canary-runs.sh Outdated
jeremy added a commit that referenced this pull request May 27, 2026
… rule

Post-launch reconciliation for the pairwise BC4<->BC5 canary. Clears the 14
review threads on #308 and corrects the memories rule to match what BC5
master actually shipped.

scripts/compare-canary-runs.sh:
- P0 (codex/cubic): match the TS live runner's snapshot filename scheme
  exactly -- sanitize the test name with [^A-Za-z0-9_-]+ -> "_" (case
  preserved), not lowercase/hyphen/operation-first. The old form matched no
  files, so check-bc5-compat could report clean without comparing anything.
- P1 (cubic): a declared pairwise test missing a snapshot on either backend
  is now a hard error (exit 2), not a silent skip-and-pass.
- empty `paths` loop (copilot): guard ${#RULE_PATHS[@]} instead of
  "${RULE_PATHS[@]:-}" so an empty array can't run against the body root.
- pairwiseEqual (cubic): compare via jq deep-equality so object key-order
  differences don't false-fail.
- exit-code contract (copilot): snapshot-read jq failures remap to the
  documented exit 2 instead of leaking jq's status under set -e.

conformance/schema.json: pairwiseAssertions[].paths gains minItems:1.

Makefile:
- rm -rf "$(LIVE_RECORD_DIR)" guarded against empty / "/" / "..".
- conformance-live validates LIVE_RECORD_DIR/BASECAMP_BACKEND before the TS
  live pass (invoked from the recipe after the guards) rather than after it.
- check-bc5-compat comment: "reports all violations", not "fails on first".
- new check-compare-canary target, wired into `make check`.

.github/workflows/live-canary.yml: default BASECAMP_HOST in the workflow
expression so an unset optional secret can't clobber the Makefile default.

CONTRIBUTING.md: align the "all violations" wording; reframe the memories
example to the waiver pattern.

memories canary rule (conformance/tests/live-my-surface.json): the prior
"BC3 commit 64acf34 aliases memories[] to bubble_ups[]" claim is
counterfactual -- that commit does not exist, and BC5 master shipped
`json.memories []` while BC4 four still populates it. The
pairwiseSupersetArray rule now states the intended invariant and is waived by
a temporary pairwiseDeltaAllowed entry pointing at
spec/api-gaps/memories-emptied-regression.md (added on the reconciliation
branch); remove the waiver once BC3 #10947 (which carries
`json.memories @bubble_ups`) merges.

scripts/test-compare-canary-runs.sh: network-free regression coverage for the
filename scheme (incl. proof the old form finds nothing), missing-snapshot
hard-fail, memories-waiver scoping, pairwiseEqual key-order, and the
empty-paths guard.
@jeremy
jeremy requested a review from Copilot May 27, 2026 23:14
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 27, 2026
@github-actions github-actions Bot removed the documentation Improvements or additions to documentation label May 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread conformance/tests/live-my-surface.json Outdated
Comment thread CONTRIBUTING.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 7 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread conformance/tests/live-my-surface.json Outdated
jeremy added a commit that referenced this pull request May 27, 2026
The waiver `reason` strings read "BC4 four still populates it", which scans as
a typo to anyone without the post-launch topology in hand (BC4 is the `four`
branch). Reword to "BC4 (the four branch)" in the live-my-surface.json
description + pairwiseDeltaAllowed reason and the CONTRIBUTING.md example.
Addresses the Copilot re-review on PR #308.
@jeremy
jeremy requested a review from Copilot May 27, 2026 23:20
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 27, 2026
@github-actions github-actions Bot removed the documentation Improvements or additions to documentation label May 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

jeremy added a commit that referenced this pull request May 28, 2026
…s branch

The brief's SDK-absorption runbook read as if the pairwiseDeltaAllowed waiver
and conformance/tests/live-my-surface.json were present here, but those canary
files are on PR #308's branch (this branch is the registry record only).
Reword to state #308 introduces the waiver and that it gets removed from
live-my-surface.json once #308 and BC3 #10947 both land. Also reframe the
"no Smithy change" bullet to "no structural change" — this PR does realign the
memories doc comment (documentation, not a shape change).
jeremy added a commit that referenced this pull request May 28, 2026
COORDINATION.md said the pairwiseSupersetArray rule "is waived via
pairwiseDeltaAllowed", which reads as if the canary surface exists on this
branch — it does not (conformance/tests/live-my-surface.json is on PR #308).
Reword to attribute the rule + waiver to #308's branch, matching the same fix
already made in the memories-emptied-regression brief.
jeremy added a commit that referenced this pull request Jul 14, 2026
…s branch

The brief's SDK-absorption runbook read as if the pairwiseDeltaAllowed waiver
and conformance/tests/live-my-surface.json were present here, but those canary
files are on PR #308's branch (this branch is the registry record only).
Reword to state #308 introduces the waiver and that it gets removed from
live-my-surface.json once #308 and BC3 #10947 both land. Also reframe the
"no Smithy change" bullet to "no structural change" — this PR does realign the
memories doc comment (documentation, not a shape change).
jeremy added a commit that referenced this pull request Jul 14, 2026
COORDINATION.md said the pairwiseSupersetArray rule "is waived via
pairwiseDeltaAllowed", which reads as if the canary surface exists on this
branch — it does not (conformance/tests/live-my-surface.json is on PR #308).
Reword to attribute the rule + waiver to #308's branch, matching the same fix
already made in the memories-emptied-regression brief.
@jeremy
jeremy force-pushed the bc5-readiness-wire-replay-decoders branch from bb85531 to 0d64a65 Compare July 14, 2026 02:41
jeremy added 23 commits July 21, 2026 22:01
The bullet suggested pages[*] was for how-many-pages checks, but
pairwiseSupersetArray compares the total item count across pages
(absent page values contribute 0, non-array page values are invalid).
Say so, so operators don't write rules against the old semantics.
schema.json requires a reason on pairwiseDeltaAllowed, but the compare
script also runs standalone (check-bc5-compat, the scheduled workflow)
with no schema-validation step in front of it — a reason-less waiver
still flattened into ALLOW_PATHS and suppressed enforcement unaudited.
The extraction now rejects waivers without a non-empty string reason
(exit 2). Scenario R pins it; scenario P's assertion follows the
combined waiver error message.
A LIVE_RECORD_DIR (or, theoretically, a mktemp result) beginning with
'-' would be parsed as an rm option; end option parsing explicitly.
The compare + harness scripts guard for it with a clear error; the
Makefile comment now says so up front for contributors running
make check on stock macOS bash.
…waiver reason

* Each snapshot's recorded .operation must match the test entry's
  operation before comparison — a stale or overwritten file with the
  right name exits 2 instead of being compared as this test's capture.
  Scenario S pins it.

* conformance/schema.json aligns with the runtime waiver guard: the
  pairwiseDeltaAllowed conditional now sets minLength 1 on reason, so
  an empty-string reason fails schema validation the same way it fails
  the compare script. TS conformance suite re-run green (84 passed).
schema.json's minItems:1 on paths doesn't run in front of standalone
invocations (check-bc5-compat, the scheduled workflow). An empty waiver
paths array is functionally harmless (it waives nothing) but is always
a fixture mistake — reject it with the same exit-2 operator error as
the other waiver shape violations. Scenario T pins it.
… message

* The rm -rf guard now refuses '..' only as a PATH SEGMENT (leading,
  interior, trailing, or the whole path) instead of any substring, so
  benign names like tmp/live-canary..pr308 are allowed while every
  traversal form still refuses. Patterns unit-verified for all six
  shapes.

* The regression harness preflights jq with the same clear error as
  the compare script instead of failing mid-setup with a raw
  command-not-found.

* snapshot_read_error's hint no longer blames only malformed JSON —
  an invalid jq path expression built from a rule's path entry lands
  in the same handler.
The aggregate is a list, not an object — the rule flags it as an
invalid target rather than evaluating it. Say so and point rule
authors at pages[N].body.X for keys checks on paginated endpoints.
The aggregate handling keyed off any '[*]' substring, broader than the
documented pages[*].body.X form — an undocumented items[*].foo would
stream through jq with undefined comparison semantics instead of
surfacing the fixture mistake. Both the read and the supersetArray
totals branch now see only validated leading-pages[*] paths; anything
else exits 2 as an unsupported path. Scenario U pins it.
* CONTRIBUTING's prerequisites now list the shared tooling make check
  needs beyond per-SDK toolchains: jq, and bash >= 4.4 for the
  pairwise-canary scripts (macOS /bin/bash is 3.2 — brew install bash;
  the scripts fail fast with that exact hint).

* Scenario U2 pins that a second '[*]' after a valid leading pages[*]
  is rejected as unsupported (the guard already covered it).
* An absolute LIVE_RECORD_DIR equal to the repo checkout (or any of
  its ancestors, e.g. LIVE_RECORD_DIR=$PWD or $HOME) passed the
  guards and rm -rf'd the working tree before the canary ran. A new
  prefix check refuses any path the checkout lives under; absolute
  paths elsewhere (/tmp/canary) and in-checkout subdirs still work.
  Patterns unit-verified for checkout, $HOME, /tmp, relative, and
  in-checkout-absolute shapes.

* The harness's mktemp gets an explicit template — current macOS
  accepts a bare -d (empirically verified here), but older BSD
  variants insist on a template and it costs nothing.
… divergence

bc3's BC5 API train shipped 2026-07-18..21 and settled the memories question:
doc/api/sections/my_notifications.md (bc3 #11628) documents memories as an
always-empty placeholder superseded by bubble_ups, and bc3 #10947 — the
once-planned `json.memories @bubble_ups` alias — is closed unmerged. The
pairwiseDeltaAllowed entry is therefore not a temporary waiver pending a fix;
it is the permanent machine-readable record of an accepted BC4→BC5
subtractive delta.

Update the GetMyNotifications test description and both rule reasons in
live-my-surface.json, and align the canonical example + prose in
CONTRIBUTING.md, replacing the 'drop the waiver once #10947 merges' guidance
with the actual retire conditions: BC4 empties memories too, or BC5's
documented contract changes.
is_allowed's early continue ran before the '[*]' syntax guard, so an
unsupported path (items[*].foo, a double star) that also appeared in a
pairwiseDeltaAllowed waiver exited clean instead of reporting the fixture
error — a typo in a waived canary rule could permanently suppress
enforcement without ever surfacing. Validate path syntax first, then apply
the allowlist skip. Adds scenario U3 covering the waived-unsupported-path
case. Flagged by Codex review.
The unknown-RULE_TYPE rejection lived in the dispatch case inside the
per-path loop, which only runs for paths that survive the is_allowed skip.
A misspelled type (pairwiseSupersetAray) whose paths were all waived by
pairwiseDeltaAllowed therefore exited clean — an operator error silently
suppressed, and the typo'd rule never enforced anything again. Hoist the
type check to rule extraction, before any per-path processing; the case's
catch-all arm stays as defense in depth. Adds scenario V covering the
waived-misspelled-type case. Flagged by Codex review.
The star guard only matched the literal '[*]', so a bare jq-stream typo
like items[] (or pages[0].body.items[]) reached read_value unwrapped: the
stream made the length variables multi-line/empty, the -lt comparison
errored falsy, and a real shrink violation (BC4 [1,2] vs BC5 []) exited 0
— a false-green on exactly the invariant the script enforces.

Generalize the guard: brackets are only accepted as a leading pages[N] or
pages[*] segment (regex-validated), everything else is rejected as a
fixture mistake with exit 2, still ahead of the waiver skip. Adds
scenario W reproducing the items[] false-green. Flagged by Codex review.
- extract validate_rule_path() and apply it to pairwiseDeltaAllowed
  entries too: a malformed bracket path appearing only in a waiver names
  a path no enforcing rule can ever target and previously sat unnoticed
- require bracketed paths to enter the page body (pages[N].body...):
  page-level keys other than body are backend-specific metadata with no
  pairwise semantics, and a typo like pages[0].items read null on both
  snapshots and false-greened superset rules
- validate_snapshot now type-checks page keys (status number, headers
  object, bodyText string, url string) so typed corruption fails loud
  instead of reading as nulls
- check-bc5-compat rm guard: canonicalize an existing LIVE_RECORD_DIR
  (cd + pwd -P) before the checkout-ancestor check so non-canonical
  spellings can't slip past the string compare; a failed cd empties LRD,
  which the ancestor pattern then refuses

Regression scenarios X (waiver-only malformed path), Y (bracket path
skipping .body), Z (wrongly-typed page keys) added; 29/29 green.
A concrete pages[N] rule asserts the page exists on both backends. When
either capture had fewer pages, read_value returned null and the rule
passed clean — hiding a fixture typo (pages[1] against single-page
captures) or a genuine page-count regression. Both demand operator
attention: fail with exit 2 naming the short snapshot and its page
count. Cross-page-count comparison remains the job of pages[*]
aggregation.

Regression scenarios AA (absent from both) and AB (one-sided page loss,
error names the bc5 snapshot) added; 31/31 green.
…kets

A path with jq punctuation but no brackets ('items, .pages') slipped the
bracket blacklist and interpolated into the jq program as raw syntax.
Terminate the class: validate_rule_path now admits only the documented
grammar — dotted [A-Za-z0-9_-] key segments, optionally led by a
pages[N]/pages[*] indexer that must enter the page body, plus the ""
body-root sentinel — and rejects everything else as a fixture mistake.

Regression scenario AC (comma path) added; 32/32 green.
…snapshots

- validate_rule_path: remove '-' from the segment charset — '.foo-bar'
  parses in jq as subtraction ('.foo - bar'), not a key lookup, so a
  hyphen-keyed rule evaluated to garbage instead of reading the field.
  No fixture path uses hyphens; keys requiring quoting are a future,
  deliberate extension of to_jq_path. Regression scenario AD added
  (33/33 green).
- live-canary.yml: upload only the decode/ result trees. Wire snapshots
  carry raw response bodies and captured headers from a live account and
  must not persist as downloadable workflow artifacts; decode results
  carry only booleans, error strings, and dotted field paths.
The round-7 canonicalization rewrote LRD itself, so rm -rf on a
symlinked LIVE_RECORD_DIR deleted the tree the link points to instead
of the link — a behavior change with data-loss potential. Canonicalize
into a separate guard variable for the checkout-ancestor check and keep
the rm on the original (trailing-slash-stripped) path: a symlink is
removed as a link, while a symlink pointing into the checkout is still
refused by the guard (verified both by hand).
A pairwiseEqual failure printed the complete BC4 and BC5 JSON values
into the comparison output, which the scheduled canary lands in
retained, log-access-readable Actions logs — leaking live-account field
content (names, URLs, notification text) on any drift. Report only
shape summaries (type + keys/length) and point the operator at a local
re-run for raw values. Regression scenario AE proves the violation
still fires while the content stays out of the output; 34/34 green.
Completes 5270e94: the invalid-target branches of pairwiseSupersetArray
and pairwiseSupersetKeys still embedded the full BC4/BC5 JSON values in
violation output. Factor the shape summary into a shared shape_of()
helper and use it at all three sites; no violation message carries live
field content anymore (grep-verified zero BC4=$BC4_VAL sites).
Shorthand paths ("" or "foo.bar") normalize to pages[0].body — fine
for single-page captures, but against a multi-page snapshot they
silently checked only page 0 and ignored the rest, under-enforcing the
rule. Hard-error with a pointer to pages[N]/pages[*] syntax as soon as
either side's capture has more than one page. Regression scenario AF
added; 35/35 green.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread scripts/test-compare-canary-runs.sh Outdated
The \| alternation in a plain grep pattern is a GNU basic-regex
extension; BSD grep treats it literally. grep -E with an unescaped
pipe is unambiguous everywhere.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/live-canary.yml

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b45d014575

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile
A symlink to / (or /.) passed the checkout-ancestor guard by string
accident: the canonical root makes the pattern '//' + '*', which never
matches an absolute pwd. No destructive path actually remained — rm
operates on the original path, so a root symlink lost only the link and
rm refuses trailing-dot operands — but the guard's invariant should be
explicit, not an accident of rm semantics. Refuse a canonical '/'
outright (verified by hand with a symlink to /).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conformance Conformance test suite enhancement New feature or request github-actions Pull requests that update GitHub Actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants